Add ES256 and PS256 signing algorithm support by zshenker · Pull Request #5 · fastly/rust-cat

zshenker · 2026-06-22T19:37:26Z

Summary

The Common Access Token spec supports the ES256 and PS256 signing algorithms in addition to HS256. This PR adds both signing and verification for those two asymmetric algorithms. HS256 (HMAC-SHA256) behavior is unchanged.

Algorithm	COSE id	Structure	`sign()` key	`verify()` key
`HmacSha256`	5	COSE_Mac0	raw symmetric key	raw symmetric key
`Es256`	-7	COSE_Sign1	PKCS#8 DER private	SPKI DER public
`Ps256`	-37	COSE_Sign1	PKCS#8 DER private	SPKI DER public

Changes

Algorithm enum (header.rs): adds Es256 / Ps256, updates identifier()/from_identifier().
Crypto primitives (utils.rs) via RustCrypto (p256, rsa, sha2): ES256 is ECDSA P-256 + SHA-256 (deterministic RFC 6979, 64-byte r || s signatures); PS256 is RSASSA-PSS + SHA-256/MGF1 using OsRng for the salt (WASM-friendly via getrandom).
Token wiring (token.rs): sign()/verify() dispatch per algorithm; asymmetric algorithms reuse the existing COSE_Sign1 input and emit CWT tag 61 + COSE_Sign1 tag 18.
Key format: existing &[u8] API preserved — PKCS#8 DER private keys for signing, SPKI DER public keys for verifying. New Error::InvalidKey (error.rs) for malformed keys.
Docs/example: new examples/asymmetric_signing.rs, README signing-algorithms section, updated crate docs.
Deps: bump thiserror to 2; bump rust-version to 1.88.0 — the real minimum. Because Cargo.lock isn't committed, downstream users re-resolve to the latest deps, which pull minicbor-derive 0.19.4 (uses let-chains, stabilized in 1.88) and zeroize 1.9 (edition 2024, needs 1.85). 1.85/1.86 fail to compile minicbor-derive; 1.88 builds and tests cleanly. A pinned MSRV CI job (dtolnay/rust-toolchain@1.88.0) guards against silent drift. Note this MSRV reality predates the PR: main already depended on minicbor 2.x, so its declared 1.60 was already stale — this PR just makes the metadata honest. The crypto stack uses current stable RustCrypto versions (the newer 0.14/0.10 releases are pre-1.0 release candidates).

Review-driven fixes (commit `079409e`)

Grounded in RFC 9052 (COSE structures) and RFC 9053 (algorithm registry):

COSE protected-header interop (addresses the open ES256/PS256 review threads): from_bytes() previously discarded the original protected-header bytes and sign1_input/mac0_input re-encoded the decoded header map. COSE signs the exact serialized protected bstr (RFC 9052 §4.4 — a re-encode must be byte-identical), so verifying a token whose producer used a different-but-valid CBOR encoding (map ordering, integer width) failed. Token now preserves original_protected_bytes and reuses them in sign1_input/mac0_input/to_bytes (mirroring the existing payload-bytes handling). Fixes the bug for all algorithms — HMAC had the same latent issue, masked by its Sign1/Mac0 fallback.
Single source of truth for algorithm → structure: new AlgorithmClass { Mac, Signature } + Algorithm::class() (RFC 9053 §2/§3, RFC 9052 §8.1/§8.2). to_bytes() now selects the COSE tag (Mac0=17 / Sign1=18) via an exhaustive match instead of a Some(_) catch-all, so a future MAC algorithm can no longer be silently mis-tagged as COSE_Sign1 — adding a variant is a compile error until its class is declared. is_mac() is reimplemented in terms of class().
Dedup: the byte-identical sign1_input/mac0_input collapse into one cose_input(context) helper; the context string ("Signature1"/"MAC0", RFC 9052 §4.4/§6.3) comes from AlgorithmClass::context().

Lossy-claim verification regression (commit `ac8fa56`)

Follow-up review caught a regression in the payload-bytes caching introduced above. get_payload_bytes() reused the producer's original signed payload bytes only when the full decoded map equaled the current claims projection. But Claims is a lossy projection: a registered claim carried with an unexpected CBOR type is dropped on decode — most notably aud (key 3) encoded as a CBOR array of audiences, which RFC 8392/7519 permit but RegisteredClaims.aud: Option<String> cannot hold. For such an unmutated, validly-signed external token the comparison failed, the payload was re-encoded without the dropped claim, and the genuine signature no longer matched — silently rejecting previously-accepted tokens (reject-valid, not accept-forged).

Fix: detect mutation by comparing the current claims projection against a baseline_payload_projection captured at decode time (Claims::from_map(decoded).to_map()), instead of against the full original map. Both sides drop the same information, so an unmutated lossy token still matches and keeps its byte-faithful signed bytes, while a real mutation still forces a re-encode. The eager re-encode on the cache-hit path is also dropped.
Scope: correctness only — the array aud now verifies and round-trips byte-faithfully, but is still not readable via the typed audience() accessor. Exposing it would change RegisteredClaims.aud's type (breaking), so it's deferred to a future breaking release.
Tests: test_aud_as_array_verifies_and_roundtrips, test_cti_as_text_verifies_and_roundtrips (verify + byte-faithful round-trip; fail without the fix), and test_aud_as_array_mutation_is_reflected (mutation on a lossy token is still reflected and breaks the original MAC).

Tests (tests.rs)

Round-trip sign/verify, COSE_Sign1 tag bytes, wrong-key & cross-algorithm rejection, payload tampering, PSS salt randomization, invalid-key errors, identifier round-trips, the class()/context() mapping, a non-canonical protected-header regression test (mints an external ES256 token with alg=-7 in 2-byte form and asserts both verify and a byte-faithful to_bytes() round-trip; fails without the interop fix), and the lossy-claim round-trip regression tests above.

Verification

cargo test — 50 lib tests + 73 doctests pass
cargo clippy --all-targets --all-features -- -D warnings — clean
cargo fmt --check — clean
cargo run --example asymmetric_signing — ES256 and PS256 both sign and verify

🤖 Generated with Claude Code

The Common Access Token spec supports the ES256 and PS256 signing algorithms in addition to HS256. This adds both signing and verification for those asymmetric algorithms. - Algorithm enum gains Es256 (COSE -7) and Ps256 (COSE -37) plus an is_mac() helper to distinguish COSE_Mac0 from COSE_Sign1 algorithms - Signing/verification primitives via RustCrypto (p256, rsa, sha2); ES256 uses deterministic RFC 6979, PS256 uses OsRng for the PSS salt - token sign()/verify() dispatch per algorithm; asymmetric algorithms use COSE_Sign1 with CWT tag 61 + tag 18 - Keys reuse the existing &[u8] API: PKCS#8 DER private keys for signing, SPKI DER public keys for verifying; new Error::InvalidKey for malformed keys - 13 new tests covering round-trip, wrong-key/cross-algorithm rejection, payload tampering, PSS salt randomization, and invalid-key handling - New asymmetric_signing example, README signing-algorithms table, and updated crate docs - Bump thiserror to 2 and rust-version to 1.72 (required by RustCrypto) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Generates ES256 and PS256 tokens with a ~10-year expiration and prints them in hex and base64url, alongside their public keys. Each token is self-verified (signature and claims) before printing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

This PR extends the Common Access Token crate to support asymmetric signing and verification using ES256 (P-256 ECDSA) and PS256 (RSA-PSS), in addition to the existing HS256 (HMAC-SHA256) support. It updates the algorithm model, token encoding/verification dispatch, cryptographic primitives, and adds tests/examples/docs to cover the new functionality.

Changes:

Add Algorithm::{Es256, Ps256} (+ COSE identifiers) and use is_mac() to select COSE_Mac0 vs COSE_Sign1.
Implement ES256/PS256 signing + verification utilities using RustCrypto (p256, rsa, sha2, rand_core).
Add asymmetric round-trip tests and new examples/docs demonstrating key formats and COSE tags.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/utils.rs`	Adds ES256/PS256 compute + verify helpers with PKCS#8/SPKI DER key parsing.
`src/token.rs`	Tags tokens as COSE_Mac0 (HS256) vs COSE_Sign1 (ES256/PS256) and dispatches sign/verify accordingly.
`src/tests.rs`	Adds asymmetric algorithm tests (round-trips, tag bytes, wrong-key, tampering, invalid-key errors).
`src/header.rs`	Extends `Algorithm` enum with identifiers and `is_mac()` helper.
`src/constants.rs`	Adds COSE algorithm identifier constants for ES256/PS256.
`src/error.rs`	Introduces `Error::InvalidKey` for malformed DER keys.
`src/lib.rs`	Updates crate-level docs to mention ES256/PS256 support.
`README.md`	Documents the new algorithms, key formats, and COSE_Sign1 usage.
`examples/asymmetric_signing.rs`	Adds an example of signing/verifying with ES256/PS256.
`examples/sample_es256_ps256_tokens.rs`	Adds an example to mint and print sample ES256/PS256 tokens.
`Cargo.toml`	Adds crypto dependencies and bumps `rust-version` / `thiserror`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    let wrong_public_key = ct_codecs::Base64::decode_to_vec(
+        "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEqM7q0vY3RfQ8vJpV4hQ4Z0H8K7m2xZ1k9d0c8N3vWf6m1pQ2rX5sT8uY9wA0bC1dE2fG3hI4jK5lM6nO7pPqQ==",
+        None,
+    );
+    // Even if decoding the bogus key fails or it's structurally invalid, the
+    // point is verification must not succeed against the real signature.
+    let token = build_signed_token(Algorithm::Es256, &private_key);
+    let token_bytes = token.to_bytes().expect("Failed to encode token");
+    let decoded = Token::from_bytes(&token_bytes).expect("Failed to decode token");
+
+    if let Ok(wrong_key) = wrong_public_key {
+        assert!(
+            decoded.verify(&wrong_key).is_err(),
+            "Verification should fail with a non-matching public key"
+        );
+    }


+    // Flip a byte near the end of the payload region (before the signature).
+    let idx = token_bytes.len() - 70;
+    token_bytes[idx] ^= 0xFF;


+const ES256_PRIVATE_KEY_B64: &str = "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg7BOlgwBOMKscTUCaG3RmlSCgUznDdxMn+9Pvoqp4pUOhRANCAARWMcvR3DnF1U15IvgcOyAxr3pJPfOHcF7ESuY+H+ya3LCH03PC1d99/XgN1ldF+wmMxVhY0w9iop10N6tNZDTg";
+const ES256_PUBLIC_KEY_B64: &str = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEVjHL0dw5xdVNeSL4HDsgMa96ST3zh3BexErmPh/smtywh9NzwtXfff14DdZXRfsJjMVYWNMPYqKddDerTWQ04A==";
+
+const PS256_PRIVATE_KEY_B64: &str = "MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCHjA5iauwvo2sRB529iV1c+p+WuGFzk5EUGFFLYoIHxAwo/rSmZ2/D00epwb4WzOxA4c8+1QA+0rZIN35Fti9Wiunt0b1DgC0tuSglNzpEE5gjhTDcAWZOBPOCMt9pKEuuQC4eqBRxPoG5Y14dVi46/aQOQSqU5I0T3cbeLliTzjXkrvdqySFXMGpM9/I469SZRxZbDgB8wUcB2nTIuwOokjN/Vp+BpMM5QmR66J6aFNi8LqCmQv3grUI1kM1fqrC3az/YcyXcDvjinagyXsGYgW2ZpXIf2760UXv/bASAOO01sgI8zxbIDdG6Vd+7iPhr4b/v6QIj6rpuURFfns2LAgMBAAECggEAH9CdXbdYCZRzYHNnsGGqEtVWmQNdCEo2Lr/IcQfFmnoHGqYyE67Kmm2gb/VkHyjpOQ9nXAmVvakqlMfFsSoicU84uhPVNx9CO22uwRF18R2iQ5ATGEiR0TUzTLeRHbcSEGvLB3IPHkd8Hl327K7aOglntNrR2lHM1UFkWKkLLGHObPoLBSTQLjX5JkvtpUuBgnPVlfBUc5al9+CH+m/SiC4BvVWo4hiHEKCQgMIQ/Dh8UtS9Vk91FIizqKpqBXE6+PNmAnn9ZwRjZoRNBSLn0paAyiEXXdr5rV8zeYU0ktY40J9qWEFOJmTYII4pUK1U8tukrQ0w4LUm17f8zMkufQKBgQC7MGcSrbFWVjlEwA760sG6NKOZb5sL+2etIVAJyfSoGrwr8H4aQA1WFP+pmmlCWsLZj8qfTYSyocwfT/p9aY9Na7ftyks+q1QSsDF+D7frgxmITJeCSwiPa7jnOTrmReqAEOyPn8IlytHIhJbaPxzDxPf572QIAIBgsWhdygn21QKBgQC5X9agS0u2Joypz36ZIilbgbtgmSvFAE/22U0il+3GgXQbjmxPCip1UZm1cBgmLhq12bxU1xYxJpGVPWhEsmkIrOkEfNf/RYlSvVLzbuZLQxeB1g5FDrFb1EbaegrFznv/rFonyXMeRyJ7PHtDttfN5jxNTxTiV3BQ4uobgsai3wKBgFEiW6q26mSXnt7zuApzi1CgPEDnJPb+kyNxivWTOZ4baHBLHv1VwfILy/zBVtpR6J7QOmzt9pROmOEBk3sEY/6Ur/Y7dn3FWP14rRsMyRUlj82KFSl+SEmR0WU3YxYoO8oii8Z84nPrAx68iX4zWM5p82m7n0nwnbRLcQcl6Ue5AoGACjVN42viEnjS/DLx/MrVzjU5tVsZ/vJCdQyIY+RL8seENlREgKHFrso8lbJDki6tx9/isCVcEn7WO4qzKD1O7WxgNKAPYP5aTpUgcUllIzXhoIPCK2lguPbapANefoAdcfnyyQgd78fpDTJKc3MpNSx9m6BEPSalh77HN5afC68CgYBSHR2vz1GuUzHSgU+3xKqGSc+jlroetJ1dC5913Z+9eawW7QrRfmSod+JfEiJSw8eS+5/rGYjKihMtNPyqzadRvZtp0QGZrrm1k1/vqqeeH5Uq6AgH/2Djql4tUvC3gmgpHjY7RyPDv6v+u+L9C6MP0Nu5vVfQwpAmX9bsjn/Tjw==";
+const PS256_PUBLIC_KEY_B64: &str = "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAh4wOYmrsL6NrEQedvYldXPqflrhhc5ORFBhRS2KCB8QMKP60pmdvw9NHqcG+FszsQOHPPtUAPtK2SDd+RbYvVorp7dG9Q4AtLbkoJTc6RBOYI4Uw3AFmTgTzgjLfaShLrkAuHqgUcT6BuWNeHVYuOv2kDkEqlOSNE93G3i5Yk8415K73askhVzBqTPfyOOvUmUcWWw4AfMFHAdp0yLsDqJIzf1afgaTDOUJkeuiemhTYvC6gpkL94K1CNZDNX6qwt2s/2HMl3A744p2oMl7BmIFtmaVyH9u+tFF7/2wEgDjtNbICPM8WyA3RulXfu4j4a+G/7+kCI+q6blERX57NiwIDAQAB";


+const ES256_PRIVATE_KEY_B64: &str = "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg7BOlgwBOMKscTUCaG3RmlSCgUznDdxMn+9Pvoqp4pUOhRANCAARWMcvR3DnF1U15IvgcOyAxr3pJPfOHcF7ESuY+H+ya3LCH03PC1d99/XgN1ldF+wmMxVhY0w9iop10N6tNZDTg";
+const ES256_PUBLIC_KEY_B64: &str = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEVjHL0dw5xdVNeSL4HDsgMa96ST3zh3BexErmPh/smtywh9NzwtXfff14DdZXRfsJjMVYWNMPYqKddDerTWQ04A==";
+
+const PS256_PRIVATE_KEY_B64: &str = "MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCHjA5iauwvo2sRB529iV1c+p+WuGFzk5EUGFFLYoIHxAwo/rSmZ2/D00epwb4WzOxA4c8+1QA+0rZIN35Fti9Wiunt0b1DgC0tuSglNzpEE5gjhTDcAWZOBPOCMt9pKEuuQC4eqBRxPoG5Y14dVi46/aQOQSqU5I0T3cbeLliTzjXkrvdqySFXMGpM9/I469SZRxZbDgB8wUcB2nTIuwOokjN/Vp+BpMM5QmR66J6aFNi8LqCmQv3grUI1kM1fqrC3az/YcyXcDvjinagyXsGYgW2ZpXIf2760UXv/bASAOO01sgI8zxbIDdG6Vd+7iPhr4b/v6QIj6rpuURFfns2LAgMBAAECggEAH9CdXbdYCZRzYHNnsGGqEtVWmQNdCEo2Lr/IcQfFmnoHGqYyE67Kmm2gb/VkHyjpOQ9nXAmVvakqlMfFsSoicU84uhPVNx9CO22uwRF18R2iQ5ATGEiR0TUzTLeRHbcSEGvLB3IPHkd8Hl327K7aOglntNrR2lHM1UFkWKkLLGHObPoLBSTQLjX5JkvtpUuBgnPVlfBUc5al9+CH+m/SiC4BvVWo4hiHEKCQgMIQ/Dh8UtS9Vk91FIizqKpqBXE6+PNmAnn9ZwRjZoRNBSLn0paAyiEXXdr5rV8zeYU0ktY40J9qWEFOJmTYII4pUK1U8tukrQ0w4LUm17f8zMkufQKBgQC7MGcSrbFWVjlEwA760sG6NKOZb5sL+2etIVAJyfSoGrwr8H4aQA1WFP+pmmlCWsLZj8qfTYSyocwfT/p9aY9Na7ftyks+q1QSsDF+D7frgxmITJeCSwiPa7jnOTrmReqAEOyPn8IlytHIhJbaPxzDxPf572QIAIBgsWhdygn21QKBgQC5X9agS0u2Joypz36ZIilbgbtgmSvFAE/22U0il+3GgXQbjmxPCip1UZm1cBgmLhq12bxU1xYxJpGVPWhEsmkIrOkEfNf/RYlSvVLzbuZLQxeB1g5FDrFb1EbaegrFznv/rFonyXMeRyJ7PHtDttfN5jxNTxTiV3BQ4uobgsai3wKBgFEiW6q26mSXnt7zuApzi1CgPEDnJPb+kyNxivWTOZ4baHBLHv1VwfILy/zBVtpR6J7QOmzt9pROmOEBk3sEY/6Ur/Y7dn3FWP14rRsMyRUlj82KFSl+SEmR0WU3YxYoO8oii8Z84nPrAx68iX4zWM5p82m7n0nwnbRLcQcl6Ue5AoGACjVN42viEnjS/DLx/MrVzjU5tVsZ/vJCdQyIY+RL8seENlREgKHFrso8lbJDki6tx9/isCVcEn7WO4qzKD1O7WxgNKAPYP5aTpUgcUllIzXhoIPCK2lguPbapANefoAdcfnyyQgd78fpDTJKc3MpNSx9m6BEPSalh77HN5afC68CgYBSHR2vz1GuUzHSgU+3xKqGSc+jlroetJ1dC5913Z+9eawW7QrRfmSod+JfEiJSw8eS+5/rGYjKihMtNPyqzadRvZtp0QGZrrm1k1/vqqeeH5Uq6AgH/2Djql4tUvC3gmgpHjY7RyPDv6v+u+L9C6MP0Nu5vVfQwpAmX9bsjn/Tjw==";
+const PS256_PUBLIC_KEY_B64: &str = "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAh4wOYmrsL6NrEQedvYldXPqflrhhc5ORFBhRS2KCB8QMKP60pmdvw9NHqcG+FszsQOHPPtUAPtK2SDd+RbYvVorp7dG9Q4AtLbkoJTc6RBOYI4Uw3AFmTgTzgjLfaShLrkAuHqgUcT6BuWNeHVYuOv2kDkEqlOSNE93G3i5Yk8415K73askhVzBqTPfyOOvUmUcWWw4AfMFHAdp0yLsDqJIzf1afgaTDOUJkeuiemhTYvC6gpkL94K1CNZDNX6qwt2s/2HMl3A744p2oMl7BmIFtmaVyH9u+tFF7/2wEgDjtNbICPM8WyA3RulXfu4j4a+G/7+kCI+q6blERX57NiwIDAQAB";


jedisct1 · 2026-06-23T10:06:00Z

Thanks for putting this together. I reviewed the PR description, the changed files, the existing review comments, the token/header/utils implementation around the diff, and the current CI results. This is an automated review by Swival (https://swival.dev) using only open-source models, intended to help both the PR author and the maintainers.

My recommendation is not to merge this yet.

The main blocker is that the PR advertises an MSRV that the dependency graph no longer satisfies. Cargo.toml:14 sets rust-version = "1.72.0", while Cargo.toml:24-30 adds the RustCrypto stack through p256, rsa, sha2, and rand_core. With Rust/Cargo 1.72.0 on this branch, cargo check fails before compiling this crate because the resolver selects zeroize v1.9.0, pulled through the new crypto dependencies, and that crate's manifest uses the 2024 edition, which Cargo 1.72 cannot parse.

I also verified that the branch does build and test successfully on the latest local toolchain, and cargo fmt --check plus cargo clippy --all-targets -- -D warnings pass there. That does not cover the compatibility contract in Cargo.toml, though. Since this is a library crate and Cargo.lock is not committed, downstream users building with the declared Rust version can hit the same dependency resolution failure. That makes the MSRV bump inaccurate and creates a real compatibility regression for users who rely on the crate metadata.

Before this merges, the PR should either constrain the new dependency graph so it actually builds with Rust 1.72, or raise rust-version to the real minimum supported compiler and make that intentional in the PR. If the project wants to keep the lower MSRV, a direct dependency constraint or a different crypto dependency choice may be needed, because simply having CI pass on latest stable does not prove the published crate is usable with its declared minimum.

I would keep the PR open because the core ES256 and PS256 direction appears plausible and the new tests exercise the main signing and verification paths. The compatibility issue above needs to be fixed first, and the existing comments about the embedded demo private keys in the examples are also worth addressing before merge, even though I see those as secondary to the MSRV regression.

The declared MSRV of 1.72 was inaccurate: the RustCrypto stack pulls zeroize 1.9 (edition 2024) and minicbor-derive 0.19.4 uses let-chains, so the crate cannot build below Rust 1.88 once deps resolve to latest. Since Cargo.lock is not committed, downstream users hit this directly. - Set rust-version to 1.88.0 (the real minimum) and add a pinned MSRV CI job so the contract can't silently rot again. - Add DO-NOT-USE-IN-PRODUCTION warnings next to the demo private keys in both examples. - Fix test_es256_wrong_key_fails: the old bogus key was invalid base64, so the wrong-key assertion never ran behind `if let Ok(...)`. Use a real non-matching P-256 SPKI key and decode with expect(). - Use checked_sub() in test_es256_tampered_payload_fails to avoid a panic-on-underflow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zshenker · 2026-06-24T15:38:09Z

Thanks for the thorough reviews. Pushed db41b80 addressing all the feedback.

MSRV (the blocker)

You're right that the declared 1.72 is inaccurate. I dug into the resolution and the true floor is actually higher than 1.85: because Cargo.lock isn't committed, downstream users re-resolve to the latest deps, which pulls minicbor-derive 0.19.4. That crate uses let-chains (stabilized in 1.88), and zeroize 1.9 is edition-2024 (needs 1.85). 1.85 and 1.86 both fail to compile minicbor-derive; 1.88 builds and tests cleanly.

Set rust-version = "1.88.0" — the real minimum, verified by building/testing with cargo +1.88.0 against a freshly re-resolved (latest) lockfile.
Added a pinned MSRV CI job (dtolnay/rust-toolchain@1.88.0) that builds + tests, so the declared MSRV can't silently rot again. The existing CI only ran latest stable, which is exactly why this slipped through.

Worth noting: this MSRV reality predates the PR — main already depends on minicbor 2.x, so its declared 1.60 was already stale. This PR just makes the metadata honest.

Demo keys in examples

Added prominent ⚠️ DEMO KEYS — DO NOT USE IN PRODUCTION warnings next to the private-key constants in both examples/asymmetric_signing.rs and examples/sample_es256_ps256_tokens.rs, noting they're publicly known. Kept the keys embedded so the examples remain runnable with no setup.

Test fixes (Copilot)

test_es256_wrong_key_fails: the old bogus key was malformed base64, so the wrong-key assertion never ran behind if let Ok(...). Replaced it with a real, non-matching P-256 SPKI key decoded via .expect(...), so the rejection path is always exercised.
test_es256_tampered_payload_fails: switched len() - 70 to checked_sub(70).expect(...) to avoid panic-on-underflow.

Verification

cargo fmt --check, cargo clippy --all-targets — clean
cargo test — 42 lib tests + 73 doctests pass
cargo +1.88.0 build && test with a re-resolved latest lockfile — passes; cargo +1.85.0 is correctly rejected
cargo run --example asymmetric_signing — ES256 and PS256 sign/verify

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

zshenker · 2026-06-24T15:49:13Z

 keywords = ["token", "authentication", "authorization", "cbor", "cose"]
 categories = ["authentication", "cryptography", "web-programming"]
-rust-version = "1.60.0"
+rust-version = "1.88.0"


Good catch — the PR description was stale. The code is correct at 1.88.0; I've updated the description to match and explain why (uncommitted Cargo.lock → downstream re-resolves to minicbor-derive 0.19.4 which needs let-chains/1.88, and zeroize 1.9 needs edition 2024/1.85). The pinned msrv CI job already builds+tests on 1.88.0, so all three (Cargo.toml, CI, description) are now consistent.

zshenker · 2026-06-24T15:49:11Z

+        // Apply the CWT tag (61) followed by the COSE structure tag. HMAC (MAC)
+        // algorithms use COSE_Mac0 (tag 17); asymmetric signature algorithms use
+        // COSE_Sign1 (tag 18).


Updated the comment to make the untagged case explicit (7b85884): the None branch (no algorithm in the header) intentionally emits the bare COSE array untagged, and from_bytes accepts both tagged and untagged input. The comment now documents that.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

zshenker · 2026-06-24T21:04:41Z

+    // Flip a byte near the end of the payload region (before the signature).
+    let idx = token_bytes
+        .len()
+        .checked_sub(70)
+        .expect("encoded token should be at least 70 bytes");
+    token_bytes[idx] ^= 0xFF;
+
+    // Decoding may still succeed structurally, but verification must fail.
+    if let Ok(decoded) = Token::from_bytes(&token_bytes) {
+        assert!(
+            decoded.verify(&public_key).is_err(),
+            "Verification should fail for a tampered ES256 token"
+        );
+    }


Fixed in 158bb17. The test now mutates a byte inside the custom string claim contents (XOR with 0x01 to stay in ASCII so the text stays valid UTF-8 and the CBOR structure is intact). It then asserts from_bytes succeeds before asserting verify fails, so the test can no longer pass without exercising signature rejection.

jedisct1

Thanks for the follow-up work here. I reviewed the updated PR description, the current diff, the previous discussion and review comments, the affected token/header/utils paths, the new examples and docs, and the current CI state. I also ran the branch locally with rustup run stable cargo test, rustup run stable cargo fmt --check, rustup run stable cargo clippy --all-targets -- -D warnings, and both new examples. Those all passed for me. This is an automated review by Swival (https://swival.dev) using only open-source models, intended to help both the author and the maintainers.

My recommendation is not to merge this quite yet, although I do think the ES256 and PS256 implementation direction now looks broadly reasonable and worth continuing.

The remaining blocker for me is the public compatibility story. This PR changes two public enums that are currently exhaustive: Algorithm in src/header.rs:43-50 gains Es256 and Ps256, and Error in src/error.rs:7-31 gains InvalidKey. Downstream users can legally have exhaustive matches over both of those enums today, and those users will fail to compile once these variants are added. The PR also intentionally raises rust-version from 1.60 to 1.88 in Cargo.toml:14, which is another compatibility break even though the description now explains why that is necessary.

That may all be acceptable for this feature, but it should not be accidental. Cargo.toml:3 still says version = "0.2.7", so if this is released as another 0.2.x patch, users with normal compatible-version requirements on 0.2 can pick it up and get source breaks. Before merge, I think the PR should either make this a clearly intentional 0.3.0-style breaking release, or the maintainers should separate the code change from release versioning in a way that leaves an explicit note that this cannot be shipped as a 0.2.x patch. Another option would be to first make these public enums non-exhaustive in a deliberate breaking release, but as the code stands, the compatibility impact needs to be handled directly.

There is also one smaller unresolved test-quality issue from the current review thread. test_es256_tampered_payload_fails in src/tests.rs:1595-1613 still accepts Token::from_bytes returning Err and then performs no assertion. That means the test can pass without proving that a structurally valid token with a tampered payload is rejected by ES256 verification. This is not the main reason I am requesting changes, but for a crypto behavior change it would be much better to mutate a known payload byte, assert that decoding succeeds, and then assert that verification fails.

I would keep the PR open rather than close it. The core wiring in src/token.rs now follows the existing COSE input pattern, the new helpers in src/utils.rs use the expected key formats and signature encodings, CI is green including the new MSRV job, and the earlier demo-key and stale-description feedback has been addressed. Once the compatibility/release handling is made explicit, and ideally the tamper test is tightened, I would be comfortable re-reviewing this for merge.

The MSRV bump and the new Algorithm/Error variants are source-breaking for downstream users, so this cannot ship as a 0.2.x patch: - Bump version to 0.3.0 (and README dep requirement to "0.3") to make the breaking release intentional. - Mark Algorithm and Error #[non_exhaustive] so future variant additions no longer break downstream exhaustive matches. Also tighten test_es256_tampered_payload_fails: it now mutates a byte inside the custom string claim (preserving CBOR/UTF-8 structure), asserts decoding still succeeds, and then asserts verification fails — so the test can no longer pass without exercising signature rejection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zshenker · 2026-06-24T21:04:43Z

Thanks @jedisct1 — addressed both points in 158bb17:

Compatibility / release story. You're right that this can't ship as a 0.2.x patch. I've made the breaking release explicit:

Bumped version to 0.3.0 in Cargo.toml (and the README dependency requirement to "0.3").
Additionally marked both public enums #[non_exhaustive] — Algorithm in src/header.rs and Error in src/error.rs — so that future variant additions (more algorithms, more error cases) are no longer source-breaking. Downstream exhaustive matches over these enums will now require a wildcard arm, which is the one-time break captured by the 0.3.0 bump. The MSRV 1.88 requirement is likewise covered by this minor-version breaking release.

Tamper test. Tightened test_es256_tampered_payload_fails: it now flips a byte inside the custom string claim's contents (keeping CBOR/UTF-8 valid), asserts that Token::from_bytes still succeeds, and then asserts that verify fails — so it can no longer pass without proving ES256 rejects a structurally valid but tampered payload.

cargo fmt --check, cargo clippy --all-targets -- -D warnings, and the full test suite all pass locally.

jedisct1

Thanks for the follow-up changes here. I reviewed the current PR description, the latest diff at 158bb17, the earlier discussion and review comments, and the touched implementation paths in src/header.rs, src/token.rs, src/utils.rs, src/error.rs, the new tests, examples, README, and CI workflow. This is an automated review by Swival (https://swival.dev) using only open-source models, intended to help both the pull request author and the maintainers.

My recommendation is to merge this PR now.

The previous compatibility blocker looks resolved in the current revision. Cargo.toml now makes the breaking release explicit with version = "0.3.0" and rust-version = "1.88.0", and the new MSRV job in .github/workflows/ci.yml checks that compiler version directly. The public enum changes in src/header.rs and src/error.rs are still source-breaking for users with exhaustive matches, but that is now treated as part of the 0.3.0 compatibility boundary, and marking both enums #[non_exhaustive] prevents the same kind of break from recurring for future algorithm or error additions.

The ES256 and PS256 wiring fits the existing token structure. TokenBuilder::sign now signs HMAC tokens with the existing COSE_Mac0 input and asymmetric tokens with COSE_Sign1 input, while Token::verify dispatches by the protected alg header. The new helpers in src/utils.rs use the expected key formats documented in the README and examples: PKCS#8 DER private keys for signing and SPKI DER public keys for verification. The examples also now clearly warn that the embedded private keys are demo material and publicly known.

I also rechecked the test coverage that came up in the earlier comments. The wrong-key ES256 test now uses a valid non-matching P-256 public key, and test_es256_tampered_payload_fails now mutates a byte inside a decodable payload and asserts that decoding still succeeds before verification fails. That means the tamper test is actually exercising signature rejection rather than passing because the token became malformed.

For validation, GitHub Actions shows both build and msrv passing on the latest head. Locally, I ran rustup run stable cargo fmt --check, rustup run stable cargo test, rustup run stable cargo clippy --all-targets -- -D warnings, rustup run stable cargo run --example asymmetric_signing, and rustup run stable cargo run --example sample_es256_ps256_tokens. All of those passed.

I do not see any remaining correctness, regression, or maintainability issue that should block this. The change is substantial and intentionally breaking, but the current PR makes that explicit and backs the new behavior with focused tests and runnable examples.

Adopts the Keep a Changelog format and backfills entries for the two prior tagged releases (0.2.6, 0.2.7) from git history. The unreleased 0.3.0 entry documents the ES256/PS256 support and flags the breaking changes: #[non_exhaustive] on Algorithm/Error and the MSRV bump to 1.88. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

zshenker · 2026-06-25T15:24:27Z

+            Algorithm::Es256 => {
+                let sign1_input = self.sign1_input()?;
+                verify_es256(key, &sign1_input, &self.signature)
+            }


Fixed in 079409e. The root cause was exactly this: from_bytes() decoded the protected header into a map and discarded the original bytes, then sign1_input() re-encoded that map — so a token whose producer used a different-but-valid CBOR encoding (map ordering, integer width) reconstructed a different signed input and failed.

The fix preserves the original bytes: Token now carries original_protected_bytes, populated by from_bytes(), and a new protected_bytes() helper returns those exact bytes when present (mirroring the existing original_payload_bytes handling). sign1_input(), mac0_input(), and to_bytes() all use it, so verification and re-encoding are byte-faithful to the producer's encoding per RFC 9052 §4.4 ("re-encoded byte string is identical to the decoded byte string").

This applies to all algorithms, not just ES256 — HMAC had the same latent bug, previously masked by its Sign1/Mac0 fallback. New regression test test_es256_verifies_noncanonical_protected_header mints an external token encoding alg=-7 in the non-canonical 2-byte form (0x38 0x06) and asserts both verify and a byte-faithful to_bytes() round-trip; it fails without the fix.

zshenker · 2026-06-25T15:24:36Z

+            Algorithm::Ps256 => {
+                let sign1_input = self.sign1_input()?;
+                verify_ps256(key, &sign1_input, &self.signature)
+            }


Fixed in 079409e, same fix as the ES256 thread above. PS256 verification goes through the shared sign1_input(), which now uses the preserved original protected-header bytes (original_protected_bytes on Token, via protected_bytes()) rather than re-encoding the decoded header map — so verification reproduces the exact signed protected bstr per RFC 9052 §4.4.

…ping Three related correctness/hardening changes to the COSE encoding, grounded in RFC 9052 (structures) and RFC 9053 (algorithm registry): #1 Preserve and reuse the original protected-header bytes. COSE signs the exact serialized `protected` bstr (RFC 9052 §4.4: a re-encode must be byte-identical), but `sign1_input`/`mac0_input` re-encoded the decoded header map, so verifying a token whose producer used a different-but-valid CBOR encoding (map ordering, integer width) reconstructed different signed bytes and rejected it. HMAC masked this via its Sign1/Mac0 fallback; ES256/PS256 have none. `from_bytes` now stores `original_protected_bytes` and `protected_bytes()`/`to_bytes()` reuse it, mirroring the existing payload-bytes handling. Fixes the interop bug for all algorithms. #2 Add `AlgorithmClass { Mac, Signature }` and `Algorithm::class()` as the single source of truth for the algorithm→structure mapping (RFC 9053 §2/§3, RFC 9052 §8.1/§8.2). `to_bytes()` selects the CWT-nested COSE tag (Mac0=17 / Sign1=18) via an exhaustive match instead of a `Some(_)` catch-all, so a future MAC algorithm can no longer be silently mis-tagged as COSE_Sign1 — adding a variant is now a compile error until its class is declared. `is_mac()` is reimplemented in terms of `class()`. #3 Collapse the byte-identical `sign1_input`/`mac0_input` into one `cose_input(context)` helper; the context string ("Signature1"/"MAC0", RFC 9052 §4.4/§6.3) comes from `AlgorithmClass::context()`, so #2 and #3 share the same source of truth. Tests: non-canonical protected-header verify + byte-faithful re-encode round-trip (ES256); class/context mapping. fmt + clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

zshenker · 2026-06-25T17:44:11Z

+        // 1. Protected header (encoded as CBOR and then as bstr).
+        // Reuse the original bytes for a decoded token so the re-encoding is
+        // byte-faithful to the producer's encoding (see `protected_bytes`).
+        let protected_bytes = self.protected_bytes()?;
        enc.bytes(&protected_bytes)?;


Fixed in 978ea48. to_bytes() now encodes the payload via get_payload_bytes() — the same preserved-original-bytes helper that signing and verification already use — so a decoded token's payload bstr is byte-faithful to the producer's encoding rather than re-encoded from the claims map. This closes the asymmetric gap where the protected header was preserved but the payload wasn't.

Added a round-trip regression test (test_decoded_token_reencodes_without_breaking_signature) using the external COSE_Mac0 fixture: decode → verify → to_bytes() → re-parse → verify. It fails without this fix (the re-encoded token no longer verifies) and passes with it.

jedisct1

Thanks for the latest updates. I reviewed the new commits on top of the earlier approval, the current PR discussion, the unresolved review comments, the token/header/utils changes, the examples and docs, and the current CI state. This is an automated review by Swival (https://swival.dev) using only open-source models, intended to help both the author and the maintainers.

I would not merge this revision yet. The ES256 and PS256 additions still look broadly worthwhile, and the new protected-header preservation fix is in the right direction, but it leaves the same class of round-trip bug on the payload side.

In src/token.rs:82, Token::to_bytes() still rebuilds the payload from self.claims.to_map() and encode_map(...). For tokens created by this crate that is fine, but for tokens decoded from the wire, from_bytes() already preserves original_payload_bytes, and verification uses those bytes through get_payload_bytes() at src/token.rs:693. That means a decoded token can verify correctly, then to_bytes() can emit a different payload bstr while keeping the existing signature or MAC. The re-encoded token is then invalid.

This is not just theoretical. I reproduced it with the existing external MAC0 fixture from test_mac0_token_verification_with_original_bytes in src/tests.rs:551. The decoded token verifies with testSecret, but after let reencoded = token.to_bytes()?, parsing reencoded and verifying it fails with SignatureVerification. That is a regression in a public serialization path, and it is especially risky because the new protected-header fix now makes to_bytes() appear byte-faithful for one signed field while still changing the other signed field.

The fix should be small and consistent with the rest of the PR: Token::to_bytes() should use the same preserved payload bytes that signing and verification use, probably by calling self.get_payload_bytes()? instead of rebuilding the claims map there. Please also add a regression test that decodes an externally produced token, verifies it, serializes it again, and verifies the serialized result. The existing fixture in src/tests.rs:551 is a good candidate because it already demonstrates why preserving original payload bytes matters.

I ran rustup run stable cargo fmt -- --check, rustup run stable cargo test, rustup run stable cargo clippy --all-targets -- -D warnings, and both new examples successfully. I also ran the round-trip repro described above, and that is the reason for this request for changes.

My merge recommendation is request changes for now. I would keep the PR open because the change is valuable and the remaining blocker looks focused, but this should be fixed before merging.

to_bytes() rebuilt the payload from the claims map while the protected header (fixed in 079409e) and verification already reuse the preserved original bytes. For a token decoded from the wire with a non-canonical payload encoding, this emitted a different payload bstr while keeping the original signature/MAC, producing a re-encoded token that no longer verified. Use get_payload_bytes() in to_bytes() so the payload bstr is byte-faithful to the producer's encoding, mirroring the protected-header handling. Add a round-trip regression test that decodes the external COSE_Mac0 fixture, verifies it, re-encodes via to_bytes(), and verifies the result. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zshenker · 2026-06-25T17:44:24Z

Thanks @jedisct1 — fixed in 978ea48.

Token::to_bytes() was the last place still re-encoding a signed field from its in-memory form: it rebuilt the payload from self.claims.to_map() even though from_bytes() preserves original_payload_bytes and verification reads them via get_payload_bytes(). So a decoded token could verify, then to_bytes() would emit a different payload bstr while keeping the original MAC/signature — exactly the round-trip break you reproduced with the MAC0 fixture.

The fix is the small, consistent one you suggested: to_bytes() now encodes the payload via self.get_payload_bytes()?, the same preserved-original-bytes path used for signing/verification (and mirroring the protected-header handling from 079409e). Newly built tokens are unaffected — get_payload_bytes() falls back to encoding the claims map when there are no original bytes.

I also added the regression test you recommended, built on the existing fixture at src/tests.rs:551: test_decoded_token_reencodes_without_breaking_signature decodes the external token, verifies it, re-serializes via to_bytes(), re-parses, and verifies the result. I confirmed it fails without the fix (re-encoded token fails with SignatureVerification) and passes with it.

cargo fmt --check, cargo clippy --all-targets -- -D warnings, and the full test suite (45 lib tests + 73 doctests) all pass locally.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

zshenker · 2026-06-25T17:53:08Z

+///
+/// `key` must be a PKCS#8 DER-encoded P-256 private key. The returned signature
+/// is the fixed-length 64-byte COSE representation (`r || s`), as required by the
+/// COSE specification (RFC 8152 §8.1).


Fixed in 424d67d. The doc comment now cites RFC 9053 §2.1, which defines the ECDSA r || s fixed-length signature encoding (RFC 8152 was split into RFC 9052 for the COSE structures and RFC 9053 for the algorithms). This keeps the reference consistent with the RFC 9052/9053 citations used elsewhere in the PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

zshenker · 2026-06-25T18:03:54Z

+fn test_ps256_signatures_are_randomized() {
+    // PSS uses a random salt, so two signatures over the same input differ,
+    // yet both must verify.
+    let (private_key, public_key) = ps256_keys();
+
+    let token_a = build_signed_token(Algorithm::Ps256, &private_key);
+    let token_b = build_signed_token(Algorithm::Ps256, &private_key);
+
+    assert_ne!(
+        token_a.signature, token_b.signature,
+        "PSS signatures should differ due to random salt"
+    );
+
+    token_a.verify(&public_key).expect("token_a should verify");
+    token_b.verify(&public_key).expect("token_b should verify");
+}


Fixed in d814b78. The test no longer goes through build_signed_token() (which called current_timestamp() separately per token, so the two tokens could be signed over different iat/nbf/exp claims). Both token_a and token_b are now built from a shared closure with identical, fixed claims (no current_timestamp() calls), so the signed payload is byte-for-byte the same and the PSS salt is the only source of entropy. The assert_ne! on the signatures therefore genuinely exercises salt randomization, and both still verify.

build_signed_token() called current_timestamp() per token, so token_a and token_b could be signed over different payloads (iat/nbf/exp), letting the test pass even without PSS salt randomization. Build both tokens with identical fixed claims so any signature difference comes solely from the salt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Encode both tokens with to_bytes() and assert the on-the-wire bytes share an identical prefix (tags, headers, payload, and the signature's bstr header) and differ only across the trailing signature region — proving the random PSS salt is the sole source of difference. Adds a public to_signed_payload_bytes() helper exposing the exact signed payload bstr for the premise check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zshenker · 2026-06-25T18:10:42Z

Follow-up: mutation safety for `to_bytes()` (pushed in `d814b78`, `bdae217`)

While reviewing the payload-byte preservation fix, we found a related integrity issue that goes a bit beyond the original round-trip report, and hardened it proactively.

The issue. Token.claims and Token.header are public fields. Once to_bytes() reused the cached original_payload_bytes/original_protected_bytes from a decoded token, mutating a claim after decoding and then calling to_bytes() would silently re-emit the producer's original bytes — discarding the mutation. The caller would believe they minted a token with the new claims, but the bytes on the wire carried the old ones.

The fix. get_payload_bytes() and protected_bytes() no longer blindly trust the cache. Each decodes the cached bytes and compares them by logical content against the current claims/header:

Unchanged → reuse the producer's exact original bytes (byte-faithful round-trip — the interop guarantee from the earlier fix is fully preserved).
Mutated → re-encode from the current state.

This makes the behavior fail-safe: a mutated decoded token now emits the mutated claims, and because the payload changed, the original MAC/signature no longer matches — so the token fails verification rather than passing with stale claims. A rejected token is safe; a token that verifies with the wrong claims would not be.

Tests.

test_mutated_claims_are_reflected_in_to_bytes — mutates iss on a decoded token, asserts the re-encoded token carries the mutation and fails verification with the original key.
test_unmutated_decoded_token_reuses_original_bytes — asserts a byte-identical re-encode for the unmutated case (guards the non-canonical-encoding interop guarantee).

New public API. Token::to_signed_payload_bytes() exposes the exact payload bstr covered by the signature/MAC (the same bytes signing and verification use). The PS256 randomization test uses it to assert two tokens with identical claims produce identical signed payloads, so any signature difference is attributable solely to the PSS salt.

cargo fmt --check, cargo clippy --all-targets -- -D warnings, and the full suite (47 lib tests + 73 doctests) all pass.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

    fn get_payload_bytes(&self) -> Result<Vec<u8>, Error> {
-        if let Some(ref original) = self.original_payload_bytes {
-            // Use original bytes for verification
-            Ok(original.clone())
-        } else {
-            // Encode claims for newly created tokens
-            let claims_map = self.claims.to_map();
-            encode_map(&claims_map)
+        let claims_bytes = encode_map(&self.claims.to_map())?;
+        match self.original_payload_bytes {
+            // Reuse the producer's exact bytes only when they decode to the same
+            // claims the token currently holds.
+            Some(ref original) if payload_matches(original, &self.claims) => Ok(original.clone()),
+            _ => Ok(claims_bytes),


get_payload_bytes() gated reuse of the producer's original signed payload bytes on a comparison between the full decoded map and the current claims projection. Because Claims is a lossy projection (e.g. an aud encoded as a CBOR array, which RFC 8392/7519 permit, cannot fit Option<String>), an unmutated externally-produced token failed that check, got re-encoded without the dropped claim, and no longer verified — a silent break of previously-accepted tokens. Detect mutation by comparing the current claims projection against a baseline projection captured at decode time, instead of against the full original map. Both sides drop the same information, so an unmutated lossy token still matches and keeps its byte-faithful signed bytes, while a real mutation still forces a re-encode. Also drops the eager re-encode on the cache-hit path. Add regression tests for aud-as-array and cti-as-text payloads (verify + byte-faithful round-trip), plus a lossy-token mutation test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

+        match (
+            &self.original_payload_bytes,
+            &self.baseline_payload_projection,
+        ) {
+            (Some(original), Some(baseline)) if self.claims.to_map() == *baseline => {
+                Ok(original.clone())
+            }
+            _ => encode_map(&self.claims.to_map()),
        }


zshenker · 2026-06-25T18:49:27Z

+    fn protected_bytes(&self) -> Result<Vec<u8>, Error> {
+        match self.original_protected_bytes {
+            Some(ref original) if protected_matches(original, &self.header.protected) => {
+                Ok(original.clone())
+            }
+            _ => encode_map(&self.header.protected),
+        }
+    }


The code and all the tests are compiling successfully so have you actually validated this assumption?

zshenker and others added 2 commits June 22, 2026 14:36

zshenker requested review from Copilot and jedisct1 June 22, 2026 20:48

Copilot started reviewing on behalf of zshenker June 22, 2026 20:48 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

zshenker requested a review from Copilot June 24, 2026 15:40

Copilot started reviewing on behalf of zshenker June 24, 2026 15:41 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Clarify to_bytes tagging comment for the untagged (no-algorithm) case

7b85884

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zshenker requested a review from Copilot June 24, 2026 15:50

Copilot started reviewing on behalf of zshenker June 24, 2026 15:50 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

jedisct1 requested changes Jun 24, 2026

View reviewed changes

jedisct1 approved these changes Jun 24, 2026

View reviewed changes

zshenker requested review from Copilot and jedisct1 June 24, 2026 22:16

Copilot started reviewing on behalf of zshenker June 24, 2026 22:17 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

zshenker requested a review from Copilot June 25, 2026 15:31

Copilot started reviewing on behalf of zshenker June 25, 2026 15:32 View session